-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Economic Model, Verifier Update and Cleanup #19
Conversation
…te .vscode settings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
src/near_indexer/src/helper/methods.rs
- should be inside of a generated
folder to indicate that the zkaf
package has generated this file.
This way it's clear to another developer that this is the output of zkaf
, or some build process.
There should some detail, or small mv
script to indicate that any changes to ZK Rust logic requires:
- Movement of the ELF to the
generated
folder. - Re-deployment of the Smart Contracts that reference the ELF's ImageId.
|
||
app: | ||
build: | ||
dockerfile: ./src/near_indexer/.indexer.dockerfile |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consistency between this Dockerfile and the Dockerfile elsewhere.
Consistency keeps maintainability.
if config_lines.len()==0{ | ||
return "".to_string() | ||
} | ||
let plugin_info:PluginInfo = json::serde_json::from_str(config_lines.first().unwrap().replace("BITTE_CONFIG=", "").as_str()).unwrap(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still requires formatting.
Please install rust-analyzer and ensure it's used to format your Rust code in an opinionated manner.
"./src/verifier/Cargo.toml", | ||
"./src/contract/nft/Cargo.toml", | ||
"./src/contract/verifier/contract/Cargo.toml", | ||
"./src/contract/nft/nft/Cargo.toml", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No such folder as src/contract/
It's src/contracts/
Please get better at picking up minor matters like this.
.part("pinataMetadata", Part::text("{\"name\":\"Storage SDK\",\"keyvalues\":{}}")); | ||
.part("file", Part::bytes(image).file_name("image.png")) | ||
.part("pinataOptions", Part::text("{\"wrapWithDirectory\":false}")) | ||
.part("pinataOptions", Part::text("{\"wrapWithDirectory\":false}")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In a prior review, I questioned why pinataOptions
is being used here?
Especially since the formdata is being sent to thirdweb...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a comment that references this link: https://portal.thirdweb.com/infrastructure/storage/how-storage-works
let contract_id = env::var("NEAR_CONTRACT_ADDRESS").unwrap_or(String::from("xlassixx.near")); | ||
|
||
let contract_id = env::var("NEAR_CONTRACT_ADDRESS") | ||
.unwrap_or(String::from("xlassixx.near")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should unwrap()
only here instead of defaulting to a wallet that no live environment will use...
@@ -25,7 +33,7 @@ async fn main() { | |||
//Load Essential for env Variables | |||
env::var("TWEET_BEARER").expect("TWEET_BEARER must be set"); | |||
|
|||
let nft_contract_id = env::var("NFT_CONTRACT_ID").unwrap_or("local-nft.testnet".to_owned()); | |||
let nft_contract_id = env::var("NFT_CONTRACT_ID").unwrap_or("x-bitte-nft.testnet".to_owned()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
client: &NearClient, | ||
notifier: &twitter::OathTweeterHandler, | ||
) -> Result<bool, DbErr> { | ||
let nft_contract_id = env::var("NFT_CONTRACT_ID").unwrap_or("x-bitte-nft.testnet".to_owned()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
dotenv = { workspace = true } | ||
near-contract-standards = "5.5.0" | ||
verity-verifier = { git = "https://github.com/usherlabs/verity-dp", rev = "030110e32d1bccca5f2dd5108c6db1d6dc6694cd", version = "0.0.0", default-features = false, features = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure all dependencies share the same version/revision - to prevent errors.
ie. What if the Client uses the Verifier aswell, but the version of Client is different this Verifier.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: Publisher is simply used to test the ELF binary, and that the ZK Prover is working.
id:"".to_string(), | ||
username: Some("".to_string()), | ||
created_at:"".to_string() | ||
})).username |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Format please
No description provided.